batch::get-many should parallel store.get
#55
Conversation
devigned
force-pushed
the
get-many-fix
branch
from
8d9ff50 to
071dcdd
Compare
Signed-off-by: David Justice <[email protected]>
devigned
force-pushed
the
get-many-fix
branch
from
071dcdd to
60f63ec
Compare
| /// MAY show an out-of-date value if there are concurrent writes to the store. | ||
| /// | ||
| /// If any other error occurs, it returns an `Err(error)`. | ||
| get-many: func(bucket: borrow<bucket>, keys: list<string>) -> result<list<option<tuple<string, list<u8>>>>, error>; | ||
| get-many: func(bucket: borrow<bucket>, keys: list<string>) -> result<list<tuple<string, option<list<u8>>>>, error>; |
There was a problem hiding this comment.
Can you please update the docs for get-many?
/// get-many returns the key-value pairs associated with the keys in the store. It returns a list of
/// key-value pairs.
/// The returned list of key-value pairs must have the same length as that of the `keys` parameter.
/// If any of the keys do not exist in the store, it returns a (key, `none`) value for that pair in the list.
|
I can go either way on this. I think we may have had it the way you have it in this PR before, but I swapped it out because you have to return a copy of the key. For a batch request, that means even if you have no data to show, you have to return a new key (which could be quite large for some people). Since batch requests are big, this is where extra allocations of keys could matter. I swapped it to the option because it should be returned in the same order as passed in, so if you absolutely need to know which key you need, you can refer to that. Here are our options and the tradeoffs: What you have in this PR: Advantages: Easiest to consume because you have each KV pair Current: Advantages: No extra allocations of non-existent keys Option 3 Advantages: No extra allocation of keys So I think those are the options. After putting some thought into it here, I'd rather have option 3 to avoid all sorts of extra memory usage (and copying), but we should decide together what we'd prefer here |
in store returns an
option<list<u8>>, butin batch returns
list<option<tuple<string, list<u8>>>>. I would expectget-manyto returnresult<list<tuple<string, option<list<u8>>>>, error>;.